Skip to content

fix(rp2040): wait for delayed Windows CDC handoff#1095

Merged
zackees merged 1 commit into
mainfrom
fix/1074-rp2040-cdc-handoff
Jul 16, 2026
Merged

fix(rp2040): wait for delayed Windows CDC handoff#1095
zackees merged 1 commit into
mainfrom
fix/1074-rp2040-cdc-handoff

Conversation

@zackees

@zackees zackees commented Jul 16, 2026

Copy link
Copy Markdown
Member

Fixes #1074

Restores the normal post-flash success path for delayed Windows CDC enumeration: wait up to 30 seconds using the existing boards catalogue and serial selector, perform a final catalogue scan at the deadline, and retain #1083's confirmed-flash fallback with detailed timeout diagnostics.

Validation:

  • soldr cargo clippy -p fbuild-deploy -p fbuild-serial --all-targets -- -D warnings
  • soldr cargo test -p fbuild-deploy --lib
  • soldr cargo test -p fbuild-serial --lib

Summary by CodeRabbit

  • Bug Fixes

    • Improved post-deployment detection of application serial ports across supported boards.
    • Added board-specific wait times to better handle delayed USB CDC availability.
    • Deployment failures now provide clearer diagnostics, including detected ports and elapsed wait time.
    • Confirmed flashes with delayed port detection are reported with an appropriate warning instead of an immediate failure.
  • Documentation

    • Updated USB CDC handoff timing guidance for supported board families.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

RP2040 deployment now uses board-family application CDC timing, polls with injectable clocks, performs final catalogue-aware timeout handling, and reports detailed diagnostics. Board timing tables, documentation, and tests were updated for delayed CDC enumeration.

Changes

RP2040 CDC recovery

Layer / File(s) Summary
Board handoff timing and timeout policy
crates/fbuild-serial/src/boards.rs, crates/fbuild-deploy/src/rp2040.rs, docs/usb-cdc-control-line-matrix.md
HandoffTiming now defines application CDC wait budgets, including 30 seconds for RP2040/SAMD 1200-bps touch targets; RP2040 deployment derives its post-deploy timeout from this policy.
CDC polling, diagnostics, and result resolution
crates/fbuild-deploy/src/rp2040.rs
CDC polling uses CDC_POLL_INTERVAL and an injectable clock, timeout errors carry elapsed-time and candidate diagnostics, and post-flash results distinguish confirmed from unconfirmed flash outcomes. Tests cover delayed matching, deadline scans, diagnostics, and ambiguous selection.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

Possibly related PRs

Sequence Diagram(s)

sequenceDiagram
  participant Rp2040Deployer
  participant CdcWatcher
  participant BoardCatalogue
  Rp2040Deployer->>CdcWatcher: Wait for runtime CDC port
  CdcWatcher->>BoardCatalogue: Poll catalogue candidates
  BoardCatalogue-->>CdcWatcher: Matching port or timeout candidates
  CdcWatcher-->>Rp2040Deployer: Port or timeout diagnostics
  Rp2040Deployer->>Rp2040Deployer: Resolve confirmed or unconfirmed flash
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the RP2040 delayed CDC handoff fix.
Linked Issues check ✅ Passed The changes match #1074 by adding bounded board-family handoff timing, final-scan recovery, detailed timeout diagnostics, and deterministic tests.
Out of Scope Changes check ✅ Passed The board-timing and docs updates are directly tied to the RP2040 CDC recovery work, with no obvious unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1074-rp2040-cdc-handoff

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zackees
zackees merged commit 83a5018 into main Jul 16, 2026
84 of 93 checks passed
@zackees
zackees deleted the fix/1074-rp2040-cdc-handoff branch July 16, 2026 12:35

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/fbuild-deploy/src/rp2040.rs`:
- Around line 1127-1140: Update both timeout return paths in the RP2040
post-flash CDC handling: the CdcWaitError::Timeout(diagnostics) arm with
flash_confirmed should append diagnostics.diagnostics() to the
PostFlashCdc::Unconfirmed message, and the other timeout arm should append it to
the FbuildError::DeployFailed message. Keep the existing timeout guidance intact
while exposing the diagnostics to daemon/API clients.
- Around line 2194-2197: Update the test
rp2040_post_flash_cdc_policy_exceeds_the_legacy_15_second_window so it does not
depend on the FBUILD_RP2040_POST_DEPLOY_TIMEOUT environment variable: exercise
the timeout resolution with an injected None override or a pure
default-resolution helper, and retain the assertion that the default is 30
seconds.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2787d374-d618-4bd0-8a07-3624987a973b

📥 Commits

Reviewing files that changed from the base of the PR and between 2c24ddd and f3de4bf.

📒 Files selected for processing (3)
  • crates/fbuild-deploy/src/rp2040.rs
  • crates/fbuild-serial/src/boards.rs
  • docs/usb-cdc-control-line-matrix.md

Comment on lines +1127 to +1140
Err(CdcWaitError::Timeout(diagnostics)) if flash_confirmed => {
tracing::warn!(diagnostics = %diagnostics.diagnostics(), "RP2040 runtime CDC did not return before the confirmed-flash deadline");
Ok(PostFlashCdc::Unconfirmed(format!(
"the firmware was flashed and accepted, but the runtime CDC port did not reappear within {}s; first-plug driver installation can exceed this window — the board is likely healthy (extend the window with {POST_DEPLOY_TIMEOUT_ENV})",
window.as_secs()
))),
Err(CdcWaitError::Timeout) => Err(FbuildError::DeployFailed(format!(
)))
}
Err(CdcWaitError::Timeout(diagnostics)) => {
tracing::warn!(diagnostics = %diagnostics.diagnostics(), "RP2040 runtime CDC did not return before the unconfirmed-flash deadline");
Err(FbuildError::DeployFailed(format!(
"RP2040 firmware was transferred, but no catalogue-identified runtime CDC port appeared within {}s; verify that the firmware enables USB serial and that FastLED/boards USB data is current (extend the window with {POST_DEPLOY_TIMEOUT_ENV})",
window.as_secs()
))),
)))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Include timeout diagnostics in the returned result.

Lines 1127-1140 only write elapsed time, prior port/serial, and candidates to tracing. Daemon/API clients receiving the returned Unconfirmed note or DeployFailed error lose the required diagnostics. Append diagnostics.diagnostics() to both returned messages.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-deploy/src/rp2040.rs` around lines 1127 - 1140, Update both
timeout return paths in the RP2040 post-flash CDC handling: the
CdcWaitError::Timeout(diagnostics) arm with flash_confirmed should append
diagnostics.diagnostics() to the PostFlashCdc::Unconfirmed message, and the
other timeout arm should append it to the FbuildError::DeployFailed message.
Keep the existing timeout guidance intact while exposing the diagnostics to
daemon/API clients.

Comment on lines +2194 to +2197
#[test]
fn rp2040_post_flash_cdc_policy_exceeds_the_legacy_15_second_window() {
assert_eq!(rp2040_post_deploy_timeout(), Duration::from_secs(30));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Make the default-timeout test independent of the environment.

rp2040_post_deploy_timeout() honors FBUILD_RP2040_POST_DEPLOY_TIMEOUT, so this exact 30-second assertion fails when that supported override is present. Test an injected None override or extract a pure default-resolution helper.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/fbuild-deploy/src/rp2040.rs` around lines 2194 - 2197, Update the test
rp2040_post_flash_cdc_policy_exceeds_the_legacy_15_second_window so it does not
depend on the FBUILD_RP2040_POST_DEPLOY_TIMEOUT environment variable: exercise
the timeout resolution with an injected None override or a pure
default-resolution helper, and retain the assertion that the default is 30
seconds.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

fix(rp2040): tolerate delayed Windows CDC re-enumeration after UF2 flash

1 participant